Skip to content

Conversation

Jentsch
Copy link
Contributor

@Jentsch Jentsch commented Oct 9, 2024

This PR fixes the operators like ->, :+, =>, <- that are 'composed' from shorter operators like - and >. It also fixes operators like \/ where \ wasn't highlighted at all.

The change looks quite drastic, but the changes in the snaps look fine to me. I'd would appraciate a second pair of eye to check if an special operator is missing.

A change where I'm not sure if we want it:

   i -= 1
//  was:
//   ^ keyword.operator.arithmetic.scala
//    ^ keyword.operator.comparison.scala
//  now:
//   ^^ keyword.operator.scala <- 

One could argue, the - is realy the arithmetic operator here, as the
expressions i -= 1 most of the time means i = i - 1.

Btw. I don't think in i = 1 the = shouldn't be marked as a comparison but just as an operator.

@bishabosha
Copy link
Member

I think that because you are allowed to define def -= (foo: Int) ... that the interpretation is fine

@Jentsch
Copy link
Contributor Author

Jentsch commented Oct 9, 2024

I changed how operators are recognized and I'll update the PR text.

@Jentsch Jentsch marked this pull request as ready for review October 9, 2024 13:00
name: 'keyword.operator.arithmetic.scala'
},
{
match: '(=|\\<|>)',
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here I think we shouln't match =, as a single = doesn't compare but defines things.

Copy link
Contributor

@tgodzik tgodzik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense! You need to rebase on main to get this mergable though

Doesn't affect comparison, arithmetic nor logical.

In the match `:+` the `+` became redundant, because it's contained in
`opchar`.

Discovered while working on scala#191.
'Longer' operators like -> aren't an arithmetic operator
and than a comparison, but one operator.
With this the examples from
[the documentation](https://docs.scala-lang.org/tour/operators.html)
work.
Instead of dancing around around special operators
we can just enumerate them now. This fixes a lot of
edge cases.
@Jentsch
Copy link
Contributor Author

Jentsch commented Oct 11, 2024

Done

Copy link
Contributor

@tgodzik tgodzik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@tgodzik tgodzik merged commit f92d65a into scala:main Oct 11, 2024
2 checks passed
@Jentsch Jentsch deleted the fix-operators branch November 24, 2024 21:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants